Skip to content

test(ethexe-runtime-common): Waitlist::wait debug_assert on duplicate id (auto-tester) b64c5073de45#5523

Draft
grishasobol wants to merge 1 commit into
masterfrom
auto-tester/ethexe-runtime-common-b64c5073de45
Draft

test(ethexe-runtime-common): Waitlist::wait debug_assert on duplicate id (auto-tester) b64c5073de45#5523
grishasobol wants to merge 1 commit into
masterfrom
auto-tester/ethexe-runtime-common-b64c5073de45

Conversation

@grishasobol
Copy link
Copy Markdown
Member

@grishasobol grishasobol commented May 27, 2026

Calling the public Waitlist::wait(dispatch, expiry) method twice with the same dispatch.id panics in debug builds via debug_assert!(r.is_none()) at state.rs:666. In release builds the second call silently overwrites the first — debug and release diverge.

Scenario: duplicate_key
Hash: b64c5073de45
Unit: ethexe-runtime-common

Observed

thread 'test_waitlist_wait_duplicate_id_debug_assert' panicked at ethexe/runtime/common/src/state.rs:666:9: assertion failed: r.is_none()

Reproduces 3/3 deterministically.

Rubric items satisfied

  • (c) Panics on input the API explicitly admits as valid. Signature: pub fn wait(&mut self, dispatch: Dispatch, expiry: u32) at ethexe/runtime/common/src/state.rs:656. No documented uniqueness precondition on dispatch.id. Dispatch::reply(...) is also a pub constructor and generates a deterministic id from reply_to, so the caller can easily land on the same id twice through ordinary API usage.

Root cause

// ethexe/runtime/common/src/state.rs:656
pub fn wait(&mut self, dispatch: Dispatch, expiry: u32) {
    // ...
    let r = self.inner.insert(
        dispatch.id,
        Expiring { value: dispatch, expiry },
    );
    debug_assert!(r.is_none())   // line 666
}

BTreeMap::insert returns Some(old) on key collision; the debug_assert says "this should never happen". But:

  • The signature gives no way to know which dispatch ids are already present (no contains accessor).
  • In release the second wait silently destroys the first dispatch (the value, not just the expiry).

Suggested fixes

Pick one:

  • Return Result<(), AlreadyWaiting> and let the caller decide.
  • Add a doc-comment invariant so the contract is at least documented.
  • Make wait idempotent: refresh the expiry but reject if dispatch differs.

Practical reachability

Any actor that retries wait on the same dispatch (after wakeup, after restart from snapshot, after queueing logic that produces the same Dispatch twice) hits this. Not as edge-case as the u32::MAX-block-height bug — duplicate dispatch ids can arise from non-pathological caller mistakes.

Test source

// scenario: duplicate_key
// param_signature: Waitlist::wait twice with same dispatch.id triggers debug_assert failure
// hash: b64c5073de45

use ethexe_runtime_common::state::{Waitlist, Dispatch, PayloadLookup};
use ethexe_common::gear::MessageType;
use gprimitives::{ActorId, MessageId};
use gear_core::buffer::Payload;
use gear_core_errors::SuccessReplyReason;

#[test]
fn test_waitlist_wait_duplicate_id_debug_assert() {
    let mut waitlist = Waitlist::default();

    let dispatch = Dispatch::reply(
        MessageId::from(42),
        ActorId::from(1),
        PayloadLookup::Direct(Payload::new()),
        0,
        SuccessReplyReason::Auto,
        MessageType::Canonical,
        false,
    );

    // First wait — succeeds
    waitlist.wait(dispatch.clone(), 100);

    // Second wait with same id — debug_assert!(r.is_none()) fires in debug builds.
    // In release builds, the insert silently overwrites the previous entry.
    // Either way the behavior needs to be consistent: the key is deduplicated.
    waitlist.wait(dispatch.clone(), 200);

    // Check how many entries we have: should be 1 (second overwrote first) in release,
    // or this panics in debug due to debug_assert.
    let inner = waitlist.into_inner();
    assert_eq!(inner.len(), 1, "duplicate id should only have one entry");

    // The surviving entry should have expiry=200 (last write wins)
    let entry = inner.get(&MessageId::from(42)).expect("entry must exist");
    assert_eq!(entry.expiry, 200, "last write should win");
}

Generated by /gear-dev:tester (auto-tester). This is a draft PR — requires human review before merge.

… id (auto-tester) b64c5073de45

Calling the public Waitlist::wait(dispatch, expiry) method twice with the same
dispatch.id panics in debug builds via debug_assert!(r.is_none()) at
state.rs:666. The signature accepts (Dispatch, u32) with no documented
uniqueness precondition.

In release builds the second wait silently overwrites the first; debug and
release diverge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant